Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

image_types_qcom: create image-specific subfolders #122

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

image_types_qcom: create image-specific subfolders #122

wants to merge 1 commit into from

Conversation

igoropaniuk
Copy link
Contributor

@igoropaniuk igoropaniuk commented Dec 24, 2024 •
edited
Loading

Create image-specific subfolder in DEPLOY_DIR_IMAGE besides qcomflash
tarball archive. This allows to run QDL for device flashing
on that folder directly for local builds.

@igoropaniuk igoropaniuk mentioned this pull request Dec 24, 2024
Closed
@igoropaniuk
Copy link
Contributor Author

This is based on the suggestion in #109 (comment)

@koenkooi
Copy link
Contributor

This is the layout I was thinking about, thanks for creating this PR! I'm not entirely convinced about the bbclass split, having a flashable image now requires inheriting 2 classes. Having the pkg bbclass inherit the other class automatically would help, but wouldn't help with backward compatibility.

As much as I dislike to suggest it, having a single bbclass with a variable (e.g. QCOM_PKG_KEEP_FOLDER) controlling the deletion of the folder would give the least amount of surprises to existing users.

If we decide that lack backwards compatibility is not a show stopper, we can keep the current split and avoid having more magic variables.

@ndechesne
Copy link
Contributor

Thanks for the PR, I haven't looked at it in details yet, but for sure we should not worry about backward compatibility at this point. Now is the right time to make drastic changes that we think are needed.

ndechesne
ndechesne reviewed Dec 31, 2024
View reviewed changes
ci/base.yml Outdated
@@ -30,8 +30,8 @@ local_conf_header:
INHERIT += "buildhistory"
INHERIT += "rm_work"
qcomflash: |
IMAGE_CLASSES += "image_types_qcom"
IMAGE_FSTYPES += "qcomflash"
IMAGE_CLASSES += "image_types_qcom image_types_qcom_pkg"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

again, I haven't done a full review, and it's too late to do it in 2024 ;) but I think it would be better with a single class and fstype.

@igoropaniuk
Copy link
Contributor Author

igoropaniuk commented Jan 8, 2025 •
edited
Loading

@ndechesne
The reason why it was split on two image types is my desire to reuse image subfolder (created in create_qcomflash()) contents for the final tarball creation. As the image subfolder contains relative symlinks, I need image.bbclass to run do_image_complete() for $IMGDEPLOYDIR contents to $DEPLOY_DIR_IMAGE first for the folder with symlinks, and only after that tar will be able to follow all symlinks correctly

Please advise how that issue can be solved

@igoropaniuk igoropaniuk requested a review from ndechesne January 9, 2025 18:54
@igoropaniuk igoropaniuk changed the title [RFC] image_types_qcom: create image-specific subfolders image_types_qcom: create image-specific subfolders Jan 9, 2025
@koenkooi
Copy link
Contributor

The new one line change looks good to me!

@ndechesne
Copy link
Contributor

This looks definitely neater :)

One thing to discuss.. The OE way of doing these things, would be to have the folder name 'per build' e.g. core-image-base-qcs6490-rb3gen2-core-kit.20250110095603, and a symlink to the 'latest' build, to mimic how DEPLOY_DIR_IMAGE is typically managed. I am not sure it's needed.. but perhaps it's a good thing to keep a copy of previous build, so that we can flash back and forth things..

@igoropaniuk
Copy link
Contributor Author

igoropaniuk commented Jan 10, 2025 •
edited
Loading

@ndechesne thanks for suggestion, added symlink creation

so its now:

core-image-base-qcs6490-rb3gen2-core-kit.rootfs -> core-image-base-qcs6490-rb3gen2-core-kit.rootfs-20250110124031
core-image-base-qcs6490-rb3gen2-core-kit.rootfs-20250110124031
core-image-base-qcs6490-rb3gen2-core-kit.rootfs-20250110124031.qcomflash.tar.gz

I've also dropped do_image_qcomflash[cleandirs] = "${QCOMFLASH_DIR}", as it doesn't make sense any more

@ndechesne
Copy link
Contributor

should we append qcomflash to the folder name (and symlink)?

@igoropaniuk
image_types_qcom: create image-specific subfolders
fe6c1bd 
Create image-specific subfolder in DEPLOY_DIR_IMAGE besides qcomflash
tarball archive. This allows to run QDL for device flashing
on that folder directly for local builds.

Signed-off-by: Igor Opaniuk <[email protected]>
@igoropaniuk
Copy link
Contributor Author

I've sent a patch for fixing spdx box generator issues, that we are facing now with this change
https://lists.openembedded.org/g/openembedded-core/message/209901

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@quaresmajose quaresmajose Awaiting requested review from quaresmajose

@ricardosalveti ricardosalveti Awaiting requested review from ricardosalveti ricardosalveti is a code owner

@sbanerjee-quic sbanerjee-quic Awaiting requested review from sbanerjee-quic sbanerjee-quic is a code owner

@quic-vkraleti quic-vkraleti Awaiting requested review from quic-vkraleti quic-vkraleti is a code owner

@ndechesne ndechesne Awaiting requested review from ndechesne ndechesne is a code owner

At least 2 approving reviews are required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
Mainline Qualcomm Linux
Status: In Progress
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@igoropaniuk @koenkooi @ndechesne